Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filter_enabled and filter_condition properties #157

Merged
merged 3 commits into from
Jun 19, 2021

Conversation

gu-kevin
Copy link
Contributor

#149

Add the filter_enabled and filter_condition properties to pipeline module.

https://buildkite.com/docs/apis/rest-api/pipelines#provider-settings-properties

Copy link
Contributor

@yob yob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gu-kevin.

I've tested this manually and confirmed it sets and updates the filter_condition on a pipeline.

The acceptance spec is currently failing. I think you might need to do something like this to the test file:

diff --git a/buildkite/resource_pipeline_test.go b/buildkite/resource_pipeline_test.go
index c2f10a6..c5717e2 100644                                          
--- a/buildkite/resource_pipeline_test.go                              
+++ b/buildkite/resource_pipeline_test.go                              
@@ -69,7 +69,7 @@ func TestAccPipeline_add_remove_complex(t *testing.T) {
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.build_tags", "true"), 
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.cancel_deleted_branch_builds", "true"),
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.filter_enabled", "true"),
-                                       resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.filter_condition", "features/*"),
+                                       resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.filter_condition", "build.pull_request.id == 123"),
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.prefix_pull_request_fork_branch_names", "true"),
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.publish_blocked_as_pending", "true"),
                                        resource.TestCheckResourceAttr("buildkite_pipeline.foobar", "provider_settings.0.publish_commit_status", "true"),
@@ -330,6 +330,8 @@ func testAccPipelineConfigComplex(name string, steps string) string {
                                build_pull_requests = false
                                build_tags = true
                                cancel_deleted_branch_builds = true
+                               filter_enabled = true
+                               filter_condition = "build.pull_request.id == 123"
                                prefix_pull_request_fork_branch_names = true
                                publish_blocked_as_pending = true
                                publish_commit_status = true

It'd also be great if the new properties were added to docs/resources/pipeline.md

@gu-kevin
Copy link
Contributor Author

@yob thanks, why did the checks pass on Buildkite?

@gu-kevin gu-kevin requested a review from yob June 18, 2021 20:38
@yob yob merged commit da38de5 into buildkite:main Jun 19, 2021
@yob
Copy link
Contributor

yob commented Jun 19, 2021

Thanks!

why did the checks pass on Buildkite?

By default we don't run the full acceptance test suite on third party pull requests. Mainly because those tests use real credentials to test against a test organization on our live API, so the credentials could be used to make our lives difficult.

@yob
Copy link
Contributor

yob commented Jun 23, 2021

Thanks again for this contribution - we've just released v0.5.0 which includes this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants